-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support loading trust root CAs on Linux #136
Conversation
5.9 (nightly) and main nightly crashes because of this compiler bug: swiftlang/swift#68708 |
} | ||
|
||
@usableFromInline | ||
var _certificates: _TinyArray<Element> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there ever more than two elements here? If not, can we use an enum instead of a TinyArray
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of allowing to combing certificate stores which would in theory allow up to 3 elements i.e. [.customCertificates(...), .trustRoots, .customCertificates(...)]
. The current public API doesn't allow that though and I don't have a concrete use case in mind for that. Do you think something like that is useful or not? If not, we can go with a tuple. I think it makes the implementation a bit more complicated as we can't iterate over tuples but hopefully it will not be too bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think sticking with the tuple is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now implemented _TinyArray2
which store up to two elements inline.
this package seems to be experiencing a second, seemingly different compiler crash as well: swiftlang/swift#68767 , which affects documentation generation. |
# Conflicts: # Benchmarks/Package.swift # Package.swift
@tayloraswift thanks for reporting! The crash you have discovered is even on 5.9-release, mine is only on 5.9 nightly. I have worked around this issue for now. Not sure if we can workaround the other crash. |
API break is a false positive:
We have changed the signature but only from: public init<Certificates: Sequence>(_ certificates: Certificates) where Certificates.Element == Certificate to public init(_ certificates: some Sequence<Certificate>) which is the same thing, just with a different spelling. Bug reported: swiftlang/swift#68797 |
Sources/X509/PromiseAndFuture.swift
Outdated
} | ||
|
||
deinit { | ||
switch self.state.unsafeUnlockedValue { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this optimization is appropriate: we can just take the lock, it'll be uncontended.
@usableFromInline | ||
var _certificates: [DistinguishedName: [Certificate]] | ||
var additionTrustRoots: [DistinguishedName: [Certificate]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: "additional"
var systemTrustRoots: [DistinguishedName: [Certificate]] | ||
|
||
@usableFromInline | ||
var additionTrustRoots: [DistinguishedName: [Certificate]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: additional.
}() | ||
|
||
static let cachedSystemTrustRootsFuture: Future<[DistinguishedName: [Certificate]], any Error> = | ||
DispatchQueue.global( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer to create a serial dispatch queue for this than directly dispatching to a global queue.
Merging over API breakage checker, which is wrong here. |
Motivation
We want to eventually replace the BoringSSL X.509 layer in
swift-nio-ssl
withswift-certificates
. One missing feature inswift-certificates
is the loading of trusted root CAs from the system.Changes
CertificateStore.systemTrustRoots
that lazily loads the trusted root certificates from disk.CertificateStore.insert(_:)
and friends to add additional trusted root CAs to theCertificateStore.systemTrustRoots
CertificateStore
LockedValueBox
,Promise
andFuture
to the internal API to support lazy loading of the trust rootsResult
swift-certificates
can efficiently load the installed trusted root CAs on Linux.